-
Notifications
You must be signed in to change notification settings - Fork 582
Basic completion structure + dot completions #811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@@ -245,37 +239,6 @@ func isInTypeQuery(node *ast.Node) bool { | |||
}) != nil | |||
} | |||
|
|||
func isTypeOnlyImportDeclaration(node *ast.Node) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to ast/utilities
@@ -483,26 +446,8 @@ func isRightSideOfQualifiedNameOrPropertyAccess(node *ast.Node) bool { | |||
return false | |||
} | |||
|
|||
func getSourceFileOfModule(module *ast.Symbol) *ast.SourceFile { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to ast/utilities
@@ -957,20 +902,6 @@ func compareTypeMappers(m1, m2 *TypeMapper) int { | |||
return 0 | |||
} | |||
|
|||
func getClassLikeDeclarationOfSymbol(symbol *ast.Symbol) *ast.Node { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to ast/utilities
@@ -1800,10 +1722,11 @@ func canIncludeBindAndCheckDiagnostics(sourceFile *ast.SourceFile, options *core | |||
return isPlainJS || isCheckJS || sourceFile.ScriptKind == core.ScriptKindDeferred | |||
} | |||
|
|||
func isCheckJSEnabledForFile(sourceFile *ast.SourceFile, compilerOptions *core.CompilerOptions) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to ast/utilities.
// A reserved member name consists of the byte 0xFE (which is an invalid UTF-8 encoding) followed by one or more | ||
// characters where the first character is not '@' or '#'. The '@' character indicates that the name is denoted by | ||
// a well known ES Symbol instance and the '#' character indicates that the name is a PrivateIdentifier. | ||
func isReservedMemberName(name string) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to checker/utilities.
} | ||
return isRequireCall(initializer, true /*requireStringLiteralLikeArgument*/) | ||
} | ||
return false | ||
} | ||
|
||
func getLeftmostAccessExpression(expr *ast.Node) *ast.Node { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to ast/utilities.
@@ -6342,14 +6342,6 @@ func (p *Parser) skipRangeTrivia(textRange core.TextRange) core.TextRange { | |||
return core.NewTextRange(scanner.SkipTrivia(p.sourceText, textRange.Pos()), textRange.End()) | |||
} | |||
|
|||
func isClassMemberModifier(token ast.Kind) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to AST/utilities.
@@ -24,15 +24,6 @@ func ensureScriptKind(fileName string, scriptKind core.ScriptKind) core.ScriptKi | |||
return scriptKind | |||
} | |||
|
|||
func getLanguageVariant(scriptKind core.ScriptKind) core.LanguageVariant { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to AST/utilities.
@@ -222,62 +217,3 @@ func TestService(t *testing.T) { | |||
}) | |||
}) | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this to testutil/projecttestutil.
@@ -0,0 +1,427 @@ | |||
package checker | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file has a bunch of checker functions that are meant to be used by services.
There are existing functions in checker.go
that could probably move here later, e.g. GetSymbolAtLocation
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just skimmed it for now, have not tested it out either.
@@ -539,3 +539,21 @@ func levenshteinWithMax(s1 []rune, s2 []rune, maxValue float64) float64 { | |||
func Identity[T any](t T) T { | |||
return t | |||
} | |||
|
|||
func CheckEachDefined[S any](s []*S, msg string) []*S { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these functions used? The codecov extension shows them as uncovered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not yet. They're used in a function in completions.go
that is not currently exercised by the tests, because I haven't added enough tests yet.
JsxAttributeCompletionStyleNone JsxAttributeCompletionStyle = "none" | ||
) | ||
|
||
type UserPreferences struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long term it'll be interesting to see if we do this the same way; LSP has a different idea of how preferences are communicated than what tsserver did. But for internal stuff, probably okay.
type UserPreferences struct { | ||
// Enables auto-import-style completions on partially-typed import statements. E.g., allows | ||
// `import write|` to be completed to `import { writeFile } from "fs"`. | ||
IncludeCompletionsForImportStatements *bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these just be Tristate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They could be. To answer this and your other comment above, I haven't looked at how to handle those custom options yet via LSP. I left things as *bool
because that's closer to what we get from the LSP generated types.
} | ||
|
||
// Returns a map of all names in the file to their positions. | ||
// !!! cache this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you eventually need to do this, you'll probably just want to shove this onto the SourceFile like lineMap.
useSemicolons bool, | ||
compilerOptions *core.CompilerOptions, | ||
preferences *UserPreferences, | ||
clientOptions *lsproto.CompletionClientCapabilities, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Man, we have so many params in the old codebase
I tried running it, and I'm not getting completions. However, when I requested completion at let/*$*/ I got the following:
|
That's expected. I updated the description just now to make it more clear, but 1. only dot completions have a chance of working 2. many things are yet unimplemented and likely to error. |
Co-authored-by: Jake Bailey <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried it out and it doesn't crash, works in some cases too. LGTM 😄
This PR introduces the basic completion workflow (
getCompletionData
,completionInfoFromData
, etc), and dot completions. Global completions remain unimplemented, i.e. we're not collecting symbols for them.I haven't tested the implementation yet beyond a very basic test, and some helpers are unimplemented, but I'd like to get the code in because there's helper code that may be shared by other services that are also being ported now.